Make electric motors go in reverse better #38212
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Due to the downscaling of reverse power/speed for vehicle engines, it can happen that a vehicle is so heavy, it can move forward but not backward. Because battery-powered electric motors can turn in either direction with equal speed and torque, vehicles with electric motors should be exempt from the power-reduction for reverse speeds. This commit adds a new `vehicle::max_reverse_velocity` function as the counterpart to `vehicle::max_velocity`, where an exception is made for battery-powered motors. Here I suppose is where the maximum reverse speed multiplier for horses, bicycles, and jet turbines would eventually be made; for now it just checks for battery-powered motors. Note, how *much* the motor is contributing to overall torque is not calculated; the simple presence of any electric motor in the drivetrain (no matter how small) ought to negate the reverse-power penalty. Fixed CleverRaven#38165
|
What if vehicle has two engines, one electric and one standard, all active and then trying to go reverse? |
@SirPendrak As noted in my commit message for ddd9370, the presence of any electric motor in the drivetrain (no matter how small) will negate the reverse-power penalty. This PR does not calculate the power contribution for each motor. |
This is mainly preliminary work for adding new test cases to cover the changes I have made to maximum reverse speed; this vehicle power test seemed like a suitable place to put them. I had trouble understanding what this test was doing; refactoring it into BDD-style expressions helped me grok it while making it a whole lot easier for others to read in the future.
ZhilkinSerg
added
<Bugfix>
To minimally test the new `max_reverse_velocity` function, this commit adds two test vehicles (scooter and electric scooter), and checks that: - Combustion engine reverse velocity is 1/4 of forward - Electric engine reverse velocity is equivalent to forward Some additional refactoring of the helper functions is also done.
wapcaplet
changed the title
wapcaplet
added a commit
to wapcaplet/Cataclysm-DDA
that referenced
this pull request
Apr 2, 2020
This itype was decentralized as part of commit 3799315, but my PR for electric motors CleverRaven#38212 was made before that and needed fuel_type_battery to be defined.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
SUMMARY: Bugfixes "Give electric motors equal power in both forward and reverse"Purpose of change
Fixes #38165
Describe the solution
Adds a new
vehicle::max_reverse_velocityfunction as the counterpart tovehicle::max_velocity, checking to see if the vehicle has an active electric motor. If so, the max reverse velocity is equal to max forward velocity. Otherwise, the usual 1/4-power penalty is still applied.Describe alternatives you've considered
Considered adding a flag to all the electric motors that should be exempt from the reverse-velocity restriction, but a simple check using
has_engine_typegives what we need here.Also considered refactoring the calculations (both with cruise control on and off, since they are handled by separate functions) to be more realistic with respect to low-gear torque as opposed to high-gear maximum speed, but that's a dark hole I dared not descend for this simple bugfix.
Finally, I also considered trying to calculate the power contribution for each motor, in order to more accurately determine a maximum reverse velocity, but this too seemed like overkill for solving this problem.
Testing
Debug-spawned some vehicles, tried different gasoline and electric motors, and loaded them with large storage batteries for weight until they couldn't move any more.
Electric car with small electric motor (11 lbs, Power+7040)
Full disclosure, I had similar results with a gasoline engine - in other words, I couldn't reproduce the original bug with a gasoline-powered engine. But there, the engine power and eventual maximum weight were in the same ballpark:
Beetle with 0.4L 1-cylinder gasoline engine (44.1 lbs, Power +7370)
In other words, it is difficult to reach that edge case where the vehicle is just barely too heavy to move in reverse. Typical return values from
max_velocityfor an unloaded vehicle are in the thousands, so I think one would need to find that narrow range0 < max_velocity < 4to encounter the bug.Newly-added unit tests now verify the existing 25% reverse velocity for a combustion engine, and 100% reverse velocity for electric engines.
Additional context
Reverse velocity calculation is still extremely crude; as I noted in #38165, before this PR, all vehicle engines were simply nerfed to 1/4 velocity while in reverse, regardless of whether it's powered by pedals, electric motors, gas engines, or a horse. This seems to have an effect on both torque (accelerating from 0 on various terrain) and top speed. But the game does not yet model gearboxes or drive wheels or hybrid-electric drivetrains (or does it?), so this mathematical shortcut gives a similar result in most cases - that most vehicles can't go in reverse very quickly.
But electric motors have equal torque in either direction, so vehicles powered by electric motors should certainly be exempt from the 1/4-power nerf in the reverse direction. In theory, they can do just as well in reverse (leaving aside any questions of how difficult it might be to actually drive at high speed while looking and steering backwards - better level up that driving skill!)
This PR had some scope creep - now the majority of changes are related to unit tests and test vehicle data, and only a few lines relate to the original bugfix.